-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix #2762: Make sure constrainResult does not change context when false #2892
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
constrainResult should behave like isSubType in this respect. This fixes scala#2672. The assertion in Applications.scala is no longer needed because it is now obviously true by design.
case _ => | ||
true | ||
def constrainResult(mt: Type, pt: Type)(implicit ctx: Context): Boolean = { | ||
val savedConstraint = ctx.typerState.constraint |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it be safer to do use a new TyperState? (ctx.fresh.setNewTyperState and then commit if we want to keep it)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, except that constrainResult is called often, so copying the context each time would likely be a hit in performance. Also, the logic here is analogous to isSubType, which makes sense.
@@ -221,7 +221,7 @@ trait Applications extends Compatibility { self: Typer with Dynamic => | |||
if (ctx.typerState.isCommittable) | |||
// defer the problem until after the application; | |||
// it might be healed by an implicit conversion | |||
assert(ctx.typerState.constraint eq savedConstraint) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is removed then val savedConstraint
above is unused.
def foo()(implicit ev: Prepend): Foo[ev.Out] = ??? | ||
|
||
def test: Unit = { | ||
foo(): Foo[Any] // error: found: Prepend => Foo[_] required: Foo[Any] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't a great error message by the way, since it might be displayed instead of a more informative message based on@implicitNotFound
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But even if an implicit was found, we'd still get an error because then the function would return a Foo[_]
where a Foo[Any]
was expected.
constrainResult should behave like isSubType in this respect.
This fixes #2672. The assertion in Applications.scala is no
longer needed because it is now obviously true by design.